Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resolve precommit issue #115

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ashutosh-rath02
Copy link
Contributor

@ashutosh-rath02 ashutosh-rath02 commented Jun 19, 2023

Fixes #99
Added lint-staged to package.json as lint-staged was failing during commit
Removed cspell as the flag of cspell was raised even if there was no error.
@adithyaakrishna Please look into it.

Signed-off-by: Lucif3r-in <ashutosh123rath@gmail.com>
Copy link
Collaborator

alabulei1 commented Jun 19, 2023

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.


Commit c6311aec7723d21e150ae4fa4166e07868301d66

Key changes:

  • Removed the cspell command from .lintstagedrc.json file.
  • Added the lint-staged command in the package.json file.

Potential problems:

  • The reason for removing the cspell command is not mentioned in the Pull Request. It would be useful to know whether this was an intentional change and if it affected the project's functionality in any way.
  • It is unclear why the lint-staged command was added to the package.json file. There is no explanation in the Pull Request.

@adithyaakrishna
Copy link
Contributor

adithyaakrishna commented Jun 19, 2023

@Lucif3r-in I dont think this scrip will work, lint-staged should be seperated from scripts in package.json

It should be something like this,
Screenshot 2023-06-19 at 6 58 48 PM

The above is just an example, more info: https://www.npmjs.com/package/lint-staged & https://github.com/okonet/lint-staged

@adithyaakrishna
Copy link
Contributor

Also lint check seems to fail here, https://github.com/WasmEdge/docs/actions/runs/5312158635/jobs/9616295701?pr=115 👀

@ashutosh-rath02
Copy link
Contributor Author

@Lucif3r-in I dont think this scrip will work, lint-staged should be seperated from scripts in package.json

It should be something like this, Screenshot 2023-06-19 at 6 58 48 PM

The above is just an example, more info: https://www.npmjs.com/package/lint-staged & https://github.com/okonet/lint-staged

How do I make sure what files to check for eslint?

@ashutosh-rath02
Copy link
Contributor Author

ashutosh-rath02 commented Jun 20, 2023

@Lucif3r-in I dont think this scrip will work, lint-staged should be seperated from scripts in package.json
It should be something like this, Screenshot 2023-06-19 at 6 58 48 PM
The above is just an example, more info: https://www.npmjs.com/package/lint-staged & https://github.com/okonet/lint-staged

How do I make sure what files to check for eslint?
"lint-staged": {
'*.(js|ts){,x}': 'eslint --cache --fix --ignore-path .gitignore',
'*.{js,ts,tsx,css,md,html}': 'prettier --write',
}
@adithyaakrishna

@ashutosh-rath02
Copy link
Contributor Author

Also lint check seems to fail here, https://github.com/WasmEdge/docs/actions/runs/5312158635/jobs/9616295701?pr=115 👀

Here, I guess I had to run prettier --check

@duckling69
Copy link

@Lucif3r-in I dont think this scrip will work, lint-staged should be seperated from scripts in package.json
It should be something like this, Screenshot 2023-06-19 at 6 58 48 PM
The above is just an example, more info: https://www.npmjs.com/package/lint-staged & https://github.com/okonet/lint-staged

How do I make sure what files to check for eslint?

Look at the suggested changes its already ensuring eslint runs if .ts or .tsx files is changed here you can add js and jsx file here additionally

@ashutosh-rath02
Copy link
Contributor Author

ashutosh-rath02 commented Jun 24, 2023

@Lucif3r-in I dont think this scrip will work, lint-staged should be seperated from scripts in package.json
It should be something like this, Screenshot 2023-06-19 at 6 58 48 PM
The above is just an example, more info: https://www.npmjs.com/package/lint-staged & https://github.com/okonet/lint-staged

How do I make sure what files to check for eslint?

Look at the suggested changes its already ensuring eslint runs if .ts or .tsx files is changed here you can add js and jsx file here additionally

I think this is not a suggested change. It was an example, we have to look at the codebase and make changes accordingly. I am working on it. Thanks

@alabulei1
Copy link
Collaborator

Hi @ashutosh-rath02

Please check the CI checks. The Lint failed. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: lint-staged script missing for husky pre-commit-hook
4 participants